-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updating dependencies, operator-sdk versions to release operator #141
updating dependencies, operator-sdk versions to release operator #141
Conversation
d21edc6
to
eaee145
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions and a change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments.
1111689
to
739dc84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
@@ -2,4 +2,4 @@ dependencies: | |||
- type: olm.package | |||
value: | |||
packageName: openshift-pipelines-operator-rh | |||
version: "1.7.1" | |||
version: ">=1.5.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're supporting older versions of openshift-pipelines-operator-rh with this directive, right? Is that the goal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great question. Since we have a hard dependency on the pipelines operator, we need to may sure any version greater then 1.5.2 is in the cluster (since this is the version that has the CRD's we rely on). Ideally, as pipelines operator released new versions we'd update to the latest version, but they do not release all new versions to all OpenShift versions. So to make sure that OCO (this operator) works on all clusters we need to support 4.8 onward, this syntax makes sure we get the newest version in whatever cluster runs this operator. Hope this is clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, and the common alternative to this approach is to release multiple streams, each with a hard-pinned dependency vs. this which is more flexible but releases a single stream of this project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I'm not sure how we'd go to a multi-stream approach with a hard pinned version at this point(in a logistical sense), nor do I think we want that kind of overhead when trying to certify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments, which are mostly nits. As we discussed offline, I don't really consider myself a maintainer of this project, and so I don't know much of its operation. That said, logistically, this patch seems fine.
To that end.
/lgtm
739dc84
to
0762943
Compare
…OpenShift 4.12 Signed-off-by: Adam D. Cornett <adc@redhat.com>
0762943
to
3f24d39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
OCO wasn't working on clusters where Pipelines Operator got updated to 1.8.3. When looking into this more, different versions of OpenShift had different versions of Pipelines Operator. This PR aims to address that by switching to a
newest
dependency model, where OLM will apply the newest Pipeliens Operator that it has in a cluster. The pipeline also addressesTesting:
Note: 4.11 was not tested since the previous version was certified here, and has the same dependencies as 4.10 and 4.12
Signed-off-by: Adam D. Cornett adc@redhat.com